Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix trimming of lint docs #79750

Merged
merged 1 commit into from
Dec 10, 2020
Merged

Conversation

camelid
Copy link
Member

@camelid camelid commented Dec 5, 2020

Fixes #79748.

It was removing all the indentation before.

r? @Mark-Simulacrum

@camelid camelid added A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools labels Dec 5, 2020
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 5, 2020
@ehuss
Copy link
Contributor

ehuss commented Dec 5, 2020

Thanks for taking a look at this. The second trim was a mistake in #79522. I would kinda prefer something like the following change:

diff --git a/src/tools/lint-docs/src/lib.rs b/src/tools/lint-docs/src/lib.rs
index 326b7948098..ea54a351e03 100644
--- a/src/tools/lint-docs/src/lib.rs
+++ b/src/tools/lint-docs/src/lib.rs
@@ -143,8 +143,8 @@ fn lints_from_file(&self, path: &Path) -> Result<Vec<Lint>, Box<dyn Error>> {
                     Some((lineno, line)) => {
                         let line = line.trim();
                         if let Some(text) = line.strip_prefix("/// ") {
-                            doc_lines.push(text.trim().to_string());
-                        } else if line.starts_with("///") {
+                            doc_lines.push(text.to_string());
+                        } else if line == "///" {
                             doc_lines.push("".to_string());
                         } else if line.starts_with("// ") {
                             // Ignore comments.

@camelid
Copy link
Member Author

camelid commented Dec 6, 2020

@ehuss Could you explain why you prefer that change over this one?

@ehuss
Copy link
Contributor

ehuss commented Dec 6, 2020

It's just an opinion that it keeps the code simpler. There's no reason to differentiate between start/end trimming, so just the simple trim method is fine. Also, trim only needs to be called once, not in every if clause.

It was removing all the indentation before.

Co-authored-by: Eric Huss <eric@huss.org>
@camelid
Copy link
Member Author

camelid commented Dec 6, 2020

Ah, I think I got mixed up and thought that what you suggested wouldn't work. I applied your patch. This should be ready to merge now!

@Mark-Simulacrum
Copy link
Member

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Dec 9, 2020

📌 Commit b4b66f6 has been approved by Mark-Simulacrum

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 9, 2020
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 9, 2020
Rollup of 12 pull requests

Successful merges:

 - rust-lang#79732 (minor stylistic clippy cleanups)
 - rust-lang#79750 (Fix trimming of lint docs)
 - rust-lang#79777 (Remove `first_merge` from liveness debug logs)
 - rust-lang#79795 (Privatize some of libcore unicode_internals)
 - rust-lang#79803 (Update xsv to prevent random CI failures)
 - rust-lang#79810 (Account for gaps in def path table during decoding)
 - rust-lang#79818 (Fixes to Rust coverage)
 - rust-lang#79824 (Strip prefix instead of replacing it with empty string)
 - rust-lang#79826 (Simplify visit_{foreign,trait}_item)
 - rust-lang#79844 (Move RWUTable to a separate module)
 - rust-lang#79861 (Update LLVM submodule)
 - rust-lang#79862 (Remove tab-lock and replace it with ctrl+up/down arrows to switch between search result tabs)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 2872937 into rust-lang:master Dec 10, 2020
@rustbot rustbot added this to the 1.50.0 milestone Dec 10, 2020
@camelid camelid deleted the fix-lint-docs-trimming branch December 10, 2020 00:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rustc lint listing code snippets don't have any indentation
8 participants